-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Improved onboarding flow for the editor #2942
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughIntroduces an onboarding system: adds a context/provider, overlay, and trigger; integrates the provider into ProjectProviders; annotates UI with data-onboarding-targets; applies conditional highlight styles; adds a portal-based chat input clone during the onboarding’s initial step; adjusts stacking/positioning (z-index/relative) in several components. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant T as OnboardingTrigger
participant C as OnboardingContext
participant O as OnboardingOverlay
participant UI as UI Components<br/>(TopBar/Toolbar/Members/Chat)
participant EE as EditorEngine
U->>T: Click "Start/Restart Tour"
T->>C: startOnboarding()
activate C
C-->>UI: isActive=true, currentStep=0
C-->>O: isActive=true, currentStep=0
O->>EE: Clear selections/deselect frames
O->>UI: Query [data-onboarding-target] for step target
O-->>U: Show overlay, highlight target
U->>O: Next / Skip
O->>C: nextStep() / completeOnboarding()
C-->>UI: Update step / deactivate
O-->>U: Fade out on completion
deactivate C
sequenceDiagram
autonumber
participant C as OnboardingContext
participant CI as ChatInput (in-panel)
participant D as document.body
C-->>CI: isActive && currentStep===0
CI->>CI: Measure bounding rect
CI->>D: createPortal(clone at fixed pos)
Note over CI,D: Portal clone mirrors input UI<br/>with elevated z-index
C-->>CI: Step changes or deactivates
CI->>D: Remove portal / revert to normal render
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
startOnboarding: () => void; | ||
completeOnboarding: () => void; | ||
skipStep: () => void; | ||
setFadingOut: (value: boolean) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OnboardingContextType interface includes 'setFadingOut' but the OnboardingProvider does not supply this function. Either provide it in the context value or remove it from the interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx (1)
1-14
: Add missing 'use client' directive.This file uses
observer
,useState
, and event handlers, which require client-side execution. According to the coding guidelines, observer components must be client components.Apply this diff:
+'use client'; + import { useEditorEngine } from '@/components/store/editor'; import { transKeys } from '@/i18n/keys';
🧹 Nitpick comments (2)
apps/web/client/src/components/onboarding/onboarding-context.tsx (2)
111-116
: Add localStorage availability check.Accessing
localStorage
can throw in SSR contexts or restricted environments (e.g., private browsing). Wrap in a try-catch or check for availability.Apply this diff:
const completeOnboarding = () => { setIsActive(false); setCurrentStep(0); - // Store in localStorage that onboarding has been completed - localStorage.setItem('onlook-onboarding-completed', 'true'); + // Store in localStorage that onboarding has been completed + try { + localStorage.setItem('onlook-onboarding-completed', 'true'); + } catch (error) { + console.warn('Failed to save onboarding completion status:', error); + } };
72-95
: Wrap state-mutating functions in useCallback.
nextStep
,previousStep
,startOnboarding
,skipStep
, andcompleteOnboarding
should be wrapped inuseCallback
to prevent unnecessary re-renders of consumers and to stabilize references for the event listener useEffect.Apply this diff:
+import { createContext, useContext, useState, useEffect, useCallback } from 'react'; export const OnboardingProvider = ({ children }: OnboardingProviderProps) => { const [currentStep, setCurrentStep] = useState(0); const [isActive, setIsActive] = useState(false); const [isFadingOut, setIsFadingOut] = useState(false); - const startOnboarding = () => { + const startOnboarding = useCallback(() => { setIsActive(true); setCurrentStep(0); setIsFadingOut(false); - }; + }, []); - const nextStep = () => { + const completeOnboarding = useCallback(() => { + setIsActive(false); + setCurrentStep(0); + try { + localStorage.setItem('onlook-onboarding-completed', 'true'); + } catch (error) { + console.warn('Failed to save onboarding completion status:', error); + } + }, []); + + const nextStep = useCallback(() => { if (currentStep < ONBOARDING_STEPS.length - 1) { setCurrentStep(currentStep + 1); } else { completeOnboarding(); } - }; + }, [currentStep, completeOnboarding]); - const previousStep = () => { + const previousStep = useCallback(() => { if (currentStep > 0) { setCurrentStep(currentStep - 1); } - }; + }, [currentStep]); - const skipStep = () => { + const skipStep = useCallback(() => { nextStep(); - }; + }, [nextStep]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/client/src/app/project/[id]/_components/bottom-bar/index.tsx
(3 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx
(1 hunks)apps/web/client/src/app/project/[id]/_components/main.tsx
(3 hunks)apps/web/client/src/app/project/[id]/_components/members/index.tsx
(3 hunks)apps/web/client/src/app/project/[id]/_components/onboarding-trigger.tsx
(1 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
(6 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/index.tsx
(1 hunks)apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
(1 hunks)apps/web/client/src/app/project/[id]/_components/top-bar/mode-toggle.tsx
(1 hunks)apps/web/client/src/app/project/[id]/_components/top-bar/publish/trigger-button.tsx
(3 hunks)apps/web/client/src/app/project/[id]/providers.tsx
(2 hunks)apps/web/client/src/components/onboarding/onboarding-context.tsx
(1 hunks)apps/web/client/src/components/onboarding/onboarding-overlay.tsx
(1 hunks)apps/web/client/src/components/onboarding/red-glow.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx
: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx
apps/web/client/src/app/project/[id]/providers.tsx
apps/web/client/src/app/project/[id]/_components/bottom-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/index.tsx
apps/web/client/src/app/project/[id]/_components/onboarding-trigger.tsx
apps/web/client/src/app/project/[id]/_components/main.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/mode-toggle.tsx
apps/web/client/src/app/project/[id]/_components/members/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/publish/trigger-button.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}
: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx
apps/web/client/src/app/project/[id]/providers.tsx
apps/web/client/src/components/onboarding/onboarding-overlay.tsx
apps/web/client/src/app/project/[id]/_components/bottom-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/index.tsx
apps/web/client/src/app/project/[id]/_components/onboarding-trigger.tsx
apps/web/client/src/components/onboarding/red-glow.tsx
apps/web/client/src/app/project/[id]/_components/main.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/mode-toggle.tsx
apps/web/client/src/app/project/[id]/_components/members/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/components/onboarding/onboarding-context.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/publish/trigger-button.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx
: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx
: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx
apps/web/client/src/app/project/[id]/providers.tsx
apps/web/client/src/components/onboarding/onboarding-overlay.tsx
apps/web/client/src/app/project/[id]/_components/bottom-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/index.tsx
apps/web/client/src/app/project/[id]/_components/onboarding-trigger.tsx
apps/web/client/src/components/onboarding/red-glow.tsx
apps/web/client/src/app/project/[id]/_components/main.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/mode-toggle.tsx
apps/web/client/src/app/project/[id]/_components/members/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/components/onboarding/onboarding-context.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/publish/trigger-button.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx
apps/web/client/src/app/project/[id]/providers.tsx
apps/web/client/src/components/onboarding/onboarding-overlay.tsx
apps/web/client/src/app/project/[id]/_components/bottom-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/index.tsx
apps/web/client/src/app/project/[id]/_components/onboarding-trigger.tsx
apps/web/client/src/components/onboarding/red-glow.tsx
apps/web/client/src/app/project/[id]/_components/main.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/mode-toggle.tsx
apps/web/client/src/app/project/[id]/_components/members/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/components/onboarding/onboarding-context.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/publish/trigger-button.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx
apps/web/client/src/app/project/[id]/providers.tsx
apps/web/client/src/app/project/[id]/_components/bottom-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/index.tsx
apps/web/client/src/app/project/[id]/_components/onboarding-trigger.tsx
apps/web/client/src/app/project/[id]/_components/main.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/mode-toggle.tsx
apps/web/client/src/app/project/[id]/_components/members/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/publish/trigger-button.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx
apps/web/client/src/app/project/[id]/providers.tsx
apps/web/client/src/components/onboarding/onboarding-overlay.tsx
apps/web/client/src/app/project/[id]/_components/bottom-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/index.tsx
apps/web/client/src/app/project/[id]/_components/onboarding-trigger.tsx
apps/web/client/src/components/onboarding/red-glow.tsx
apps/web/client/src/app/project/[id]/_components/main.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/mode-toggle.tsx
apps/web/client/src/app/project/[id]/_components/members/index.tsx
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx
apps/web/client/src/components/onboarding/onboarding-context.tsx
apps/web/client/src/app/project/[id]/_components/top-bar/publish/trigger-button.tsx
🧬 Code graph analysis (9)
apps/web/client/src/app/project/[id]/providers.tsx (1)
apps/web/client/src/components/onboarding/onboarding-context.tsx (1)
OnboardingProvider
(72-133)
apps/web/client/src/components/onboarding/onboarding-overlay.tsx (2)
apps/web/client/src/components/onboarding/onboarding-context.tsx (1)
useOnboarding
(60-66)apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine
(10-14)
apps/web/client/src/app/project/[id]/_components/bottom-bar/index.tsx (1)
apps/web/client/src/components/onboarding/onboarding-context.tsx (1)
useOnboarding
(60-66)
apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx (3)
apps/web/client/src/app/project/[id]/_components/top-bar/project-breadcrumb.tsx (1)
ProjectBreadcrumb
(25-175)apps/web/client/src/app/project/[id]/_components/top-bar/branch.tsx (1)
BranchDisplay
(15-55)apps/web/client/src/app/project/[id]/_components/top-bar/mode-toggle.tsx (1)
ModeToggle
(27-87)
apps/web/client/src/app/project/[id]/_components/onboarding-trigger.tsx (1)
apps/web/client/src/components/onboarding/onboarding-context.tsx (1)
useOnboarding
(60-66)
apps/web/client/src/app/project/[id]/_components/main.tsx (2)
apps/web/client/src/app/project/[id]/_components/onboarding-trigger.tsx (1)
OnboardingTrigger
(8-23)apps/web/client/src/components/onboarding/onboarding-overlay.tsx (1)
OnboardingOverlay
(8-336)
apps/web/client/src/app/project/[id]/_components/members/index.tsx (1)
apps/web/client/src/components/onboarding/onboarding-context.tsx (1)
useOnboarding
(60-66)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-input/index.tsx (1)
apps/web/client/src/components/onboarding/onboarding-context.tsx (1)
useOnboarding
(60-66)
apps/web/client/src/app/project/[id]/_components/top-bar/publish/trigger-button.tsx (3)
apps/web/client/src/components/onboarding/onboarding-context.tsx (1)
useOnboarding
(60-66)apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine
(10-14)apps/web/client/src/components/store/hosting/type.tsx (1)
useHostingType
(6-36)
🪛 GitHub Actions: CI
apps/web/client/src/components/onboarding/onboarding-context.tsx
[error] 120-120: Type '{ currentStep: number; isActive: boolean; nextStep: () => void; previousStep: () => void; startOnboarding: () => void; completeOnboarding: () => void; skipStep: () => void; }' is missing the following properties from type 'OnboardingContextType': isFadingOut, setFadingOut
🔇 Additional comments (24)
apps/web/client/src/app/project/[id]/_components/top-bar/mode-toggle.tsx (1)
39-39
: LGTM! Clean onboarding integration.The data-onboarding-target attribute is correctly added without affecting existing functionality.
apps/web/client/src/app/project/[id]/_components/right-panel/index.tsx (1)
37-37
: LGTM! Appropriate positioning for onboarding integration.Adding
relative
positioning enables proper z-index stacking for onboarding overlays without affecting existing layout behavior.apps/web/client/src/app/project/[id]/_components/left-panel/index.tsx (1)
109-109
: LGTM! Onboarding target attribute correctly added.The data-onboarding-target attribute enables the onboarding system to locate and highlight the left panel.
apps/web/client/src/app/project/[id]/providers.tsx (2)
5-5
: LGTM! OnboardingProvider import is correct.
20-22
: LGTM! OnboardingProvider correctly integrated.The OnboardingProvider is properly nested within the existing provider hierarchy, making onboarding context available to all project components.
apps/web/client/src/app/project/[id]/_components/main.tsx (3)
20-21
: LGTM! Onboarding component imports are correct.Both imports use appropriate path aliases and reference the correct component locations.
89-90
: LGTM! OnboardingTrigger correctly positioned.The trigger button is appropriately placed in the layout after the TopBar, making it accessible to users.
129-130
: LGTM! OnboardingOverlay correctly positioned at top level.Rendering the overlay outside the main layout div ensures it can overlay all UI elements without z-index constraints from parent containers. This addresses the need for proper stacking context mentioned in the PR description.
apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx (2)
45-45
: LGTM! Z-index positioning added for onboarding.The addition of
relative z-[70]
ensures the top bar is properly positioned in the stacking context for the onboarding overlay system.
51-51
: LGTM! Onboarding target attribute added.The
data-onboarding-target="top-right-actions"
attribute enables the onboarding system to identify and highlight this UI section.apps/web/client/src/app/project/[id]/_components/members/index.tsx (3)
3-3
: LGTM! Onboarding dependencies imported.Correctly imports the onboarding context hook and the
cn
utility for conditional styling.Also applies to: 8-8
17-19
: LGTM! Onboarding state properly consumed.The component correctly derives
isOnboardingStep5
from onboarding context state. Note thatcurrentStep === 4
represents the 5th step (0-indexed).
31-38
: LGTM! Conditional onboarding highlight applied.The red focus ring and glow effect are correctly applied when the onboarding reaches step 5, maintaining consistent styling with other onboarding-aware components.
apps/web/client/src/app/project/[id]/_components/bottom-bar/index.tsx (4)
4-4
: LGTM! Onboarding context imported.Correctly imports the onboarding hook for state consumption.
61-63
: LGTM! Onboarding state consumption and derivation.The component correctly extracts onboarding state and derives
isOnboardingToolbarStep
for step 2 (currentStep === 1, 0-indexed).
75-75
: LGTM! Onboarding target attribute added.The
data-onboarding-target="project-toolbar"
attribute enables the onboarding system to identify this toolbar for highlighting.
81-84
: LGTM! Conditional onboarding styling applied.The toolbar correctly applies the red focus ring and glow effect during the onboarding's toolbar step. The
z-50
ensures proper stacking with the onboarding overlay.apps/web/client/src/app/project/[id]/_components/onboarding-trigger.tsx (1)
8-23
: LGTM! Onboarding trigger correctly implemented.The component properly consumes onboarding state and provides a user-friendly way to start or restart the tour. The conditional button text and positioning are appropriate.
apps/web/client/src/app/project/[id]/_components/top-bar/publish/trigger-button.tsx (3)
1-1
: LGTM! Onboarding context imported.Correctly imports the onboarding hook for state consumption.
12-21
: LGTM! Onboarding state properly consumed.The component correctly derives
isOnboardingStep5
from onboarding context state for step 5 highlighting (currentStep === 4, 0-indexed).
55-55
: LGTM! Conditional onboarding highlight applied.The red focus ring and glow effect are correctly applied when the onboarding reaches step 5, maintaining consistent styling with other onboarding-aware components (e.g., Members component).
apps/web/client/src/components/onboarding/onboarding-context.tsx (3)
1-4
: LGTM: Imports and client directive.The 'use client' directive and imports are correctly configured for a client-side React context.
6-11
: LGTM: OnboardingStep interface.The interface is well-defined with clear property names and appropriate types.
102-109
: Add skipStep to the useEffect dependency array.The effect closure captures
skipStep
but doesn't list it as a dependency, causing it to reference a stale version.Apply this diff:
useEffect(() => { const handleSkip = () => { skipStep(); }; window.addEventListener('onboarding-skip', handleSkip); return () => window.removeEventListener('onboarding-skip', handleSkip); - }, [skipStep]); + }, []);Actually, since
skipStep
just callsnextStep()
, and both are recreated on every render, you should either:
- Wrap
skipStep
andnextStep
inuseCallback
, or- Inline
nextStep()
directly in the handler.Better approach using inline call:
useEffect(() => { const handleSkip = () => { - skipStep(); + nextStep(); }; window.addEventListener('onboarding-skip', handleSkip); return () => window.removeEventListener('onboarding-skip', handleSkip); - }, [skipStep]); + }, [nextStep]);Then wrap
nextStep
inuseCallback
to stabilize it.Likely an incorrect or invalid review comment.
import { useOnboarding } from '@/components/onboarding/onboarding-context'; | ||
import { Button } from '@onlook/ui/button'; | ||
import { observer } from 'mobx-react-lite'; | ||
import { useEffect, useState } from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused imports.
useEffect
and useState
are imported but never used in this component.
Apply this diff:
-import { useEffect, useState } from 'react';
+import { } from 'react';
Or remove the line entirely if no React imports are needed:
-import { useEffect, useState } from 'react';
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { useEffect, useState } from 'react'; |
🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_components/onboarding-trigger.tsx
around line 6, the imports "useEffect" and "useState" are unused; remove them
from the import list (or remove the entire React import line if no other React
symbols are required) so the file no longer contains unused imports.
if (isOnboardingChatStep && containerRef.current) { | ||
const updatePosition = () => { | ||
const rect = containerRef.current?.getBoundingClientRect(); | ||
if (rect) { | ||
setPortalPosition({ | ||
top: rect.top, | ||
left: rect.left, | ||
width: rect.width, | ||
height: rect.height, | ||
}); | ||
} | ||
}; | ||
|
||
updatePosition(); | ||
window.addEventListener('resize', updatePosition); | ||
|
||
return () => window.removeEventListener('resize', updatePosition); | ||
} else { | ||
setPortalPosition(null); | ||
} | ||
}, [isOnboardingChatStep]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Portal clone never repositions with layout changes
portalPosition
is sampled only once (and on resize
), yet the clone is rendered with fixed top/left/width/height
. As soon as the chat input grows (multi-line typing, suggestion pills) or the right panel scrolls/reflows, the hidden source moves but the fixed portal stays at the stale coordinates, so the visible chat input and the onboarding glow drift apart and drops land in the wrong place. Hook a ResizeObserver
(and at least a scroll listener on the nearest scroll container) to keep portalPosition
in sync while step 0 is active.
Apply something like:
useEffect(() => {
- if (isOnboardingChatStep && containerRef.current) {
- const updatePosition = () => {
- const rect = containerRef.current?.getBoundingClientRect();
- if (rect) {
- setPortalPosition({
- top: rect.top,
- left: rect.left,
- width: rect.width,
- height: rect.height,
- });
- }
- };
-
- updatePosition();
- window.addEventListener('resize', updatePosition);
-
- return () => window.removeEventListener('resize', updatePosition);
- } else {
- setPortalPosition(null);
- }
-}, [isOnboardingChatStep]);
+ const node = containerRef.current;
+ if (!isOnboardingChatStep || !node) {
+ setPortalPosition(null);
+ return;
+ }
+
+ const updatePosition = () => {
+ const rect = node.getBoundingClientRect();
+ setPortalPosition({
+ top: rect.top,
+ left: rect.left,
+ width: rect.width,
+ height: rect.height,
+ });
+ };
+
+ const resizeObserver = new ResizeObserver(updatePosition);
+ resizeObserver.observe(node);
+ updatePosition();
+
+ const handleScroll = () => updatePosition();
+ window.addEventListener('scroll', handleScroll, true);
+
+ return () => {
+ resizeObserver.disconnect();
+ window.removeEventListener('scroll', handleScroll, true);
+ };
+}, [isOnboardingChatStep]);
Also applies to: 352-358
export const ONBOARDING_STEPS: OnboardingStep[] = [ | ||
{ | ||
id: 'chat-input', | ||
title: 'Welcome to Onlook!', | ||
description: 'Start by typing your first message here to begin designing your project.', | ||
target: 'chat-input' | ||
}, | ||
{ | ||
id: 'toolbar', | ||
title: 'Restart & Reset', | ||
description: 'Use the toolbar to restart your sandbox if you run into any issues.', | ||
target: 'editor-toolbar' | ||
}, | ||
{ | ||
id: 'left-panel', | ||
title: 'Assets & Resources', | ||
description: 'Access your assets, components, and project files in the left panel.', | ||
target: 'left-panel' | ||
}, | ||
{ | ||
id: 'design-mode', | ||
title: 'Preview Mode', | ||
description: 'Switch to design mode to see how your project looks and feels.', | ||
target: 'design-mode-toggle' | ||
}, | ||
{ | ||
id: 'publish', | ||
title: 'Share Your Work', | ||
description: 'Publish and share your amazing creations with the world!', | ||
target: 'publish-button' | ||
} | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace hardcoded text with next-intl messages.
All user-facing strings (title, description) should use next-intl messages/hooks instead of hardcoded text.
As per coding guidelines for apps/web/client/src/**/*.{ts,tsx}
: "Avoid hardcoded user-facing text; use next-intl messages/hooks instead"
Create a messages file for onboarding strings and use useTranslations
hook to access them:
+'use client';
+
+import { useTranslations } from 'next-intl';
+
+// In your component or hook:
+const t = useTranslations('onboarding');
+
-export const ONBOARDING_STEPS: OnboardingStep[] = [
- {
- id: 'chat-input',
- title: 'Welcome to Onlook!',
- description: 'Start by typing your first message here to begin designing your project.',
- target: 'chat-input'
- },
+export const getOnboardingSteps = (t: ReturnType<typeof useTranslations>): OnboardingStep[] => [
+ {
+ id: 'chat-input',
+ title: t('chatInput.title'),
+ description: t('chatInput.description'),
+ target: 'chat-input'
+ },
// ... repeat for all steps
-];
+];
Committable suggestion skipped: line range outside the PR's diff.
interface OnboardingContextType { | ||
currentStep: number; | ||
isActive: boolean; | ||
isFadingOut: boolean; | ||
nextStep: () => void; | ||
previousStep: () => void; | ||
startOnboarding: () => void; | ||
completeOnboarding: () => void; | ||
skipStep: () => void; | ||
setFadingOut: (value: boolean) => void; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Export OnboardingContextType for external consumers.
The interface should be exported so consuming code can type-check properly.
Apply this diff:
-interface OnboardingContextType {
+export interface OnboardingContextType {
currentStep: number;
isActive: boolean;
isFadingOut: boolean;
nextStep: () => void;
previousStep: () => void;
startOnboarding: () => void;
completeOnboarding: () => void;
skipStep: () => void;
setFadingOut: (value: boolean) => void;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
interface OnboardingContextType { | |
currentStep: number; | |
isActive: boolean; | |
isFadingOut: boolean; | |
nextStep: () => void; | |
previousStep: () => void; | |
startOnboarding: () => void; | |
completeOnboarding: () => void; | |
skipStep: () => void; | |
setFadingOut: (value: boolean) => void; | |
} | |
export interface OnboardingContextType { | |
currentStep: number; | |
isActive: boolean; | |
isFadingOut: boolean; | |
nextStep: () => void; | |
previousStep: () => void; | |
startOnboarding: () => void; | |
completeOnboarding: () => void; | |
skipStep: () => void; | |
setFadingOut: (value: boolean) => void; | |
} |
🤖 Prompt for AI Agents
In apps/web/client/src/components/onboarding/onboarding-context.tsx around lines
46 to 56, the OnboardingContextType interface is declared but not exported;
update the declaration to export the interface so external consumers can import
and type-check against it (i.e., change the declaration to an exported interface
and ensure any local references remain valid).
return ( | ||
<OnboardingContext.Provider | ||
value={{ | ||
currentStep, | ||
isActive, | ||
nextStep, | ||
previousStep, | ||
startOnboarding, | ||
completeOnboarding, | ||
skipStep, | ||
}} | ||
> | ||
{children} | ||
</OnboardingContext.Provider> | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add missing properties to provider value.
The provider value is missing isFadingOut
and setFadingOut
properties that are defined in the OnboardingContextType
interface, causing the pipeline failure.
Apply this diff:
<OnboardingContext.Provider
value={{
currentStep,
isActive,
+ isFadingOut,
nextStep,
previousStep,
startOnboarding,
completeOnboarding,
skipStep,
+ setFadingOut,
}}
>
{children}
</OnboardingContext.Provider>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
return ( | |
<OnboardingContext.Provider | |
value={{ | |
currentStep, | |
isActive, | |
nextStep, | |
previousStep, | |
startOnboarding, | |
completeOnboarding, | |
skipStep, | |
}} | |
> | |
{children} | |
</OnboardingContext.Provider> | |
); | |
return ( | |
<OnboardingContext.Provider | |
value={{ | |
currentStep, | |
isActive, | |
isFadingOut, | |
nextStep, | |
previousStep, | |
startOnboarding, | |
completeOnboarding, | |
skipStep, | |
setFadingOut, | |
}} | |
> | |
{children} | |
</OnboardingContext.Provider> | |
); |
🧰 Tools
🪛 GitHub Actions: CI
[error] 120-120: Type '{ currentStep: number; isActive: boolean; nextStep: () => void; previousStep: () => void; startOnboarding: () => void; completeOnboarding: () => void; skipStep: () => void; }' is missing the following properties from type 'OnboardingContextType': isFadingOut, setFadingOut
🤖 Prompt for AI Agents
In apps/web/client/src/components/onboarding/onboarding-context.tsx around lines
118 to 132, the OnboardingContext.Provider value is missing the isFadingOut and
setFadingOut properties declared on OnboardingContextType; add these two
properties to the value object (using the existing state and setter from this
component) so the provider matches the interface and the pipeline error is
resolved.
const stepContent: Record<number, string> = { | ||
0: "Type your first message here to begin designing your project.", | ||
1: "Use these tools to design and interact with your project.", | ||
2: "Access your layers, branding, pages, support, and more from here.", | ||
3: "Switch between Design and Preview modes to edit or view your project.", | ||
4: "Invite colleagues or publish your work to share it with the world.", | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Localize onboarding copy with next-intl
All of the step copy (stepContent[...]
) and button labels (Skip Tour
, Next
, Finish
) are hardcoded English strings. Per our app client guidelines we must surface user-visible text through next-intl
so it can be translated. Please move these strings into message keys (e.g. under transKeys.editor.onboarding...
) and retrieve them with useTranslations()
instead of literals. Based on coding guidelines.
Also applies to: 276-310
🤖 Prompt for AI Agents
In apps/web/client/src/components/onboarding/onboarding-overlay.tsx around lines
101-107 (and also apply the same change to lines 276-310), the onboarding step
copy and button labels are hardcoded English strings; replace these literals
with next-intl message keys (e.g. transKeys.editor.onboarding.step0 ... step4
and transKeys.editor.onboarding.skip/next/finish), import and call
useTranslations() at the top of the component to get t, and update stepContent
and all button label usages to call t('transKeys.editor.onboarding.<key>') so
text is surfaced through next-intl for translation; add the corresponding keys
to the appropriate messages file(s).
text = "Welcome to Onlook! Start by typing your first message here.", | ||
className, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Route RedGlow text through i18n
text
default (“Welcome to Onlook! …”) and the Skip
label are literal strings in a client component. Guidelines require user-facing copy to come from next-intl
so it can be translated/customized. Please replace these with localized message keys and keep the default text
prop empty, letting callers pass a translated value. Based on coding guidelines.
Also applies to: 101-118
🤖 Prompt for AI Agents
apps/web/client/src/components/onboarding/red-glow.tsx (lines 17-18, and
similarly update lines 101-118): the component currently uses hard-coded English
strings for the default text prop ("Welcome to Onlook!...") and the "Skip"
label; change the default text prop to an empty string and pull both the welcome
text and the Skip label from next-intl by using useTranslations (or passing in
translated strings from callers). Replace literal strings with
t('onboarding.welcome') and t('onboarding.skip') (or your project's message
keys), and ensure callers provide a localized text prop when needed; keep a safe
fallback (empty string) if translations are missing. Ensure imports for
next-intl/useTranslations are added and update prop types/defaultProps
accordingly.
Description
Onboarding flow for the user to go through the editor experience.
Missing a smooth transition from the first step to the second, and needs some copy improvements.
Type of Change
Important
Introduces a new onboarding flow with visual guidance and integration into existing UI components, enhancing user experience in the editor.
OnboardingProvider
anduseOnboarding
hook inonboarding-context.tsx
to manage onboarding state and steps.OnboardingOverlay
inonboarding-overlay.tsx
for visual guidance during onboarding.RedGlow
component inred-glow.tsx
for visual emphasis on elements.BottomBar
,LeftPanel
,Main
,Members
,RightPanel
,TopBar
, andChatInput
components usingdata-onboarding-target
attributes.OnboardingTrigger
component to start/restart onboarding.This description was created by
for 47439da. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Enhancements